Skip to content

chore(ci): trigger release workflow from pushes to main#185

Closed
dustinbyrne wants to merge 1 commit intomainfrom
chore/release-workflow-push-main
Closed

chore(ci): trigger release workflow from pushes to main#185
dustinbyrne wants to merge 1 commit intomainfrom
chore/release-workflow-push-main

Conversation

@dustinbyrne
Copy link
Copy Markdown
Contributor

💡 Motivation and Context

The release workflow was triggered by pull_request.closed, so protected release environments could be requested from a refs/pull/*/merge ref instead of refs/heads/main after a PR merged. That can cause environment protection to reject the release even though the PR landed on main.

This ports the CI release fix from PostHog/posthog-js#3461.

Changes

  • Trigger automatic releases from push to main instead of pull_request.closed.
  • Keep workflow_dispatch for manual releases.
  • Add a resolve-release-context job that skips automated version-bump commits, finds the PR associated with the pushed commit, and only continues when the merged PR currently has the release label.
  • Limit push-triggered releases to changeset paths where the repo uses changesets/Sampo changesets, so unrelated pushes do not queue behind the release concurrency group.
  • For bump-label release workflows, resolve the bump type from the associated PR labels and scope release concurrency to the protected release job because there is no changeset path to filter on.

💚 How did you test it?

  • Parsed .github/workflows/release.yml as YAML.
  • Ran actionlint on .github/workflows/release.yml (ignoring the pre-existing actions/create-github-app-token@v3 client-id/app-id warning where that already exists in the workflow).
  • Compared the workflow structure against the posthog-js reference PR: push trigger, release-context resolution, version-bump skip, release-label check, and gated release job.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file
  • Added the "release" label to the PR to indicate we're publishing new versions for the affected packages

@dustinbyrne dustinbyrne requested review from a team and haacked as code owners April 23, 2026 22:03
@github-actions
Copy link
Copy Markdown
Contributor

posthog-dotnet Compliance Report

Date: 2026-04-23 22:04:13 UTC
Duration: 223ms

⚠️ Some Tests Failed

0/1 tests passed, 1 failed


Feature_Flags Tests

⚠️ 0/1 tests passed, 1 failed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 43ms

Failures

request_payload.request_with_person_properties_device_id

404, message='Not Found', url='http://sdk-adapter:8080/get_feature_flag'

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 23, 2026

Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 73

Comment:
**`jq index()` returns 0 for the first label, which is falsy in jq**

`index("release")` returns the numeric position of the element. In jq, `0` is falsy, so `select(...and (.labels | map(.name) | index("release")))` silently drops PRs where `"release"` is the very first label — a realistic ordering when it's the first label added. The release would be quietly skipped with no bump type found.

The bump-label checks on lines 75–80 are safe because they use `!= null`, but this initial filter is not. Replace the truthy test with an explicit presence check:

```suggestion
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | contains(["release"])))] | first // {}')
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 101-121

Comment:
**`check-release-label` is now a passthrough with no logic of its own**

After this PR, the job only re-emits the `bump-type` and `should-release` values already set by `resolve-release-context` — its name no longer matches its behaviour and the "Has no superfluous parts" simplicity rule is violated. The only real purpose it still serves is acting as a dependency gate for `notify-approval-needed`.

Consider either removing it and wiring `notify-approval-needed` directly off `resolve-release-context`, or renaming it to something like `gate-release` to accurately describe its current role.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "chore(ci): trigger release workflow from..." | Re-trigger Greptile

fi

PRS_JSON=$(gh api "repos/${{ github.repository }}/commits/$PUSH_SHA/pulls")
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | index("release")))] | first // {}')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 jq index() returns 0 for the first label, which is falsy in jq

index("release") returns the numeric position of the element. In jq, 0 is falsy, so select(...and (.labels | map(.name) | index("release"))) silently drops PRs where "release" is the very first label — a realistic ordering when it's the first label added. The release would be quietly skipped with no bump type found.

The bump-label checks on lines 75–80 are safe because they use != null, but this initial filter is not. Replace the truthy test with an explicit presence check:

Suggested change
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | index("release")))] | first // {}')
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | contains(["release"])))] | first // {}')
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 73

Comment:
**`jq index()` returns 0 for the first label, which is falsy in jq**

`index("release")` returns the numeric position of the element. In jq, `0` is falsy, so `select(...and (.labels | map(.name) | index("release")))` silently drops PRs where `"release"` is the very first label — a realistic ordering when it's the first label added. The release would be quietly skipped with no bump type found.

The bump-label checks on lines 75–80 are safe because they use `!= null`, but this initial filter is not. Replace the truthy test with an explicit presence check:

```suggestion
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | contains(["release"])))] | first // {}')
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 101 to 121
@@ -38,29 +110,15 @@ jobs:
- name: Check release conditions
id: check
env:
EVENT_NAME: ${{ github.event_name }}
BUMP_TYPE: ${{ inputs.bump_type }}
HAS_BUMP_MAJOR: ${{ contains(github.event.pull_request.labels.*.name, 'bump-major') }}
HAS_BUMP_MINOR: ${{ contains(github.event.pull_request.labels.*.name, 'bump-minor') }}
HAS_BUMP_PATCH: ${{ contains(github.event.pull_request.labels.*.name, 'bump-patch') }}
BUMP_TYPE: ${{ needs.resolve-release-context.outputs.bump-type }}
run: |
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
if [ -n "$BUMP_TYPE" ]; then
echo "bump-type=$BUMP_TYPE" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
echo "Manual release requested with bump type '$BUMP_TYPE'"
elif [ "$HAS_BUMP_MAJOR" = "true" ]; then
echo "bump-type=major" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
elif [ "$HAS_BUMP_MINOR" = "true" ]; then
echo "bump-type=minor" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
elif [ "$HAS_BUMP_PATCH" = "true" ]; then
echo "bump-type=patch" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
echo "Release requested with bump type $BUMP_TYPE"
else
echo "should-release=false" >> "$GITHUB_OUTPUT"
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 check-release-label is now a passthrough with no logic of its own

After this PR, the job only re-emits the bump-type and should-release values already set by resolve-release-context — its name no longer matches its behaviour and the "Has no superfluous parts" simplicity rule is violated. The only real purpose it still serves is acting as a dependency gate for notify-approval-needed.

Consider either removing it and wiring notify-approval-needed directly off resolve-release-context, or renaming it to something like gate-release to accurately describe its current role.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 101-121

Comment:
**`check-release-label` is now a passthrough with no logic of its own**

After this PR, the job only re-emits the `bump-type` and `should-release` values already set by `resolve-release-context` — its name no longer matches its behaviour and the "Has no superfluous parts" simplicity rule is violated. The only real purpose it still serves is acting as a dependency gate for `notify-approval-needed`.

Consider either removing it and wiring `notify-approval-needed` directly off `resolve-release-context`, or renaming it to something like `gate-release` to accurately describe its current role.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant